Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes the FontSizePicker Custom option #18842

Merged
merged 5 commits into from Dec 3, 2019
Merged

Fixes the FontSizePicker Custom option #18842

merged 5 commits into from Dec 3, 2019

Conversation

JeanDavidDaviet
Copy link
Contributor

@JeanDavidDaviet JeanDavidDaviet commented Nov 30, 2019

Fixes #18716
This commit prevent the "Custom" option still showing up in the font sizes drop down when a WordPress theme uses the add_theme_support( 'disable-custom-font-sizes' ) feature.

Description

There was no check on the disableCustomFontSizesvariable. The custom object { slug: 'custom', name: __( 'Custom' ) } was then always added even when not wanted.

How has this been tested?

Tested with Storybook and npm test

Types of changes

Bug Fix

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.

This commit prevent the "Custom" option still showing up in the font sizes drop down when  a WordPress theme uses the `add_theme_support( 'disable-custom-font-sizes' )` feature.
The slider didn't update the select options. In the new `onSliderChangeValue` function, we set the new fontSize state with the props function `onChange` and we also set the selection option given the new font size.
This reverts commit 73f952f.
I don't know why Travis doesn't accept this commit.
Copy link
Contributor Author

@JeanDavidDaviet JeanDavidDaviet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this change makes Travis build fail. @ZebulanStanphill ?
They are necessary to update the custom select when the fontsize slider is used.

@ZebulanStanphill
Copy link
Member

@JeanDavidDaviet The automated tests haven't been working quite right lately, so the failure may be unrelated. Try recommitting the change.

@JeanDavidDaviet
Copy link
Contributor Author

@ZebulanStanphill Thanks, that was it !

@ZebulanStanphill ZebulanStanphill added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended Backport to WP Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Dec 3, 2019
@ZebulanStanphill ZebulanStanphill merged commit 68f0dc5 into WordPress:master Dec 3, 2019
@epiqueras
Copy link
Contributor

It would be good to add tests for this.

@JeanDavidDaviet
Copy link
Contributor Author

It would be good to add tests for this.

@epiqueras I won't be home for december, but I sure would like to see the test suite for this feature implemented (or modified) as I'm not a good test writer.

@JeanDavidDaviet JeanDavidDaviet deleted the fix/custom-font-size-picker-18716 branch December 5, 2019 07:33
@youknowriad youknowriad removed the Backport to WP Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Dec 9, 2019
@youknowriad youknowriad added this to the Gutenberg 7.1 milestone Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove "Custom" from font size dropdown when custom fonts are disabled
4 participants